perf: remove PureReportActionItem and use stable report#50
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Explanation of Change
Main idea
ReportActionItem/PureReportActionItemsplit (a 711-line "pure" wrapper that took every dependency as a prop) back into a singleReportActionItemthat subscribes to Onyx directly and projects its inputs through stable hooks.useStableReportForReportActionItem: narrowReportprojection over the fields the subtree actually reads.useStableReportActionForReportActionItem: narrowReportActionprojection withoriginalMessageprecomputed.ReportActionsList,MoneyRequestReportActionsList) call the stable Report hook once and pass the projection down; each item renderer builds its own stable ReportAction. Top-level Onyx ref flips driven by heartbeat fields (lastReadTime,lastMessageText,lastVisibleActionCreated, etc.) no longer cascade renders into items.indexremoved from theReportActionsListItemRendererprop signature. A newReportActionIndexContextcarries it to the rare consumer that needs it (ReportActionItemMessageEditscroll-to-index), so a position shift from a new message arriving does not invalidate items that never read the index.Reused patterns
useMemo: same shape as the old "lightweight ReportAction" memo that was inlined inReportActionsListItemRenderer, extracted into a hook and applied to both Report and ReportAction.permissionsstabilized viauseState+deepEqual(render-derived state pattern) - RC compliancemanagerID: 0collapses toundefinedso the0 → null → undefinedOnyx reconciliation does not churn the memo.Test coverage
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari